Skip to content

Handle missing quotes and leading slashes#7

Open
simonewebdesign wants to merge 4 commits intomahhov:masterfrom
simonewebdesign:main
Open

Handle missing quotes and leading slashes#7
simonewebdesign wants to merge 4 commits intomahhov:masterfrom
simonewebdesign:main

Conversation

@simonewebdesign
Copy link
Copy Markdown
Contributor

I made a couple of improvements to make the scripts work with minified files, which don't have double quotes around HTML attribute values.

Also handled paths with leading slashes by just removing them (not sure if this is a good idea in general, but it does work for my use case).

Finally I deleted bundled.html as it looks like it was committed accidentally :)

Copy link
Copy Markdown
Owner

@mahhov mahhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks for the pull request.
1 minor comment; otherwise looks good to me.


let inlineImages = async htmlPath => {
const imgTagRegex = /<img (.* )?src="([\w.\-\/]+)"(.*)>/;
const imgTagRegex = /<img (.* )?src="?([\w.\-\/]+)"?(.*)>/;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're going to relax this, then we may as well include single quotes as well.
Specifically, here and in the other files, replace "? with ['"]?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @mahhov, sorry I didn't get back to you on this. I think my change actually breaks in some cases (can't recall the exact problem now) - it happened to work fine on my minified stylesheet, but didn't really test other cases. Merging this PR at the current state is probably not a good idea, unless I dig into the problem and add some more tests!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants